Using "ExtensionAllowedTypes" = "hosted_app" results in users unable to launch bookmarks from shelf |
||||||||||||||||||||||||
Issue descriptionReported in M62, still broken in ToT. Reported here 784770 What steps will reproduce the problem? (1) Manually set up an enterprise policy with "ExtensionAllowedTypes" = "hosted_app" (2) Try to open a bookmark which was attached to shelf (via chrome settings -> more tools -> Add to shelf (3) Try to open the bookmark from the shelf What is the expected result? The bookmark opens in chrome What happens instead? The pinned shelf icon flashes, but does nothing.
,
Nov 16 2017
+Enterprise folks since this is enterprise policy. It's blocking schools from operating correctly so I'm raising priority to P1, thinking even of P0.
,
Nov 16 2017
Issue 785331 has been merged into this issue.
,
Nov 16 2017
+msw for shelf, and +khmel.
,
Nov 16 2017
I don't know how to setup an enterprise policy locally :-/ I would have suggested trying --ash-disable-shelf-model-synchronization, but that feature is off-by-default on M62. Maybe check that AppShortcutLauncherItemController::ItemSelected is calling ChromeLauncherController::LaunchApp?
,
Nov 16 2017
I have set up a device with a test policy. I enabled all extension types using "ExtensionAllowedTypes", which includes these values : "extension", "theme", "user_script", "hosted_app", "legacy_packaged_app", "platform_app" And the result is that bookmarked apps on the shelf don't launch, and launching a bookmarked app from the launcher results in a crash. I'm still digging, and I'll try logging the functions you mentioned.
,
Nov 16 2017
Bookmarked app = a bookmark which is pinned to the shelf via chrome -> settings -> more tools -> "Add to shelf..."
,
Nov 16 2017
After bookmarking a random google search and adding it to the shelf, and ensuring that device policy enables all extensions, the extensions are still being disabled. [8603:8603:1116/151819.850803:WARNING:extension_error_reporter.cc(77)] Extension error: Enterprise DeviceAttributes Test Extension (extension ID "ilnpadgckeacioehlommkaafedibdeob") is blocked by the administrator. [8603:8603:1116/151820.045815:WARNING:extension_error_reporter.cc(77)] Extension error: Chrome Sign Builder (extension ID "odjaaghiehpobimgdjjfofmablbaleem") is blocked by the administrator. [8603:8603:1116/151820.126012:WARNING:extension_error_reporter.cc(77)] Extension error: Sign-in Screen Test App (extension ID "bjaiihebfngildkcjkjckolinodhliff") is blocked by the administrator. [8603:8603:1116/151820.235104:WARNING:extension_error_reporter.cc(77)] Extension error: TPM Update Status (extension ID "nkeelpdlenncgfhkanbhagmhpaebebep") is blocked by the administrator.
,
Nov 17 2017
+test team, can you help bisect to help find when the regression was introduced?
,
Nov 17 2017
,
Nov 17 2017
It seems like newly installed web-shortcut extensions are being disabled with the extensions::disable_reason::DISABLE-REASON-LAST.
,
Nov 17 2017
Adding Benwells@ and ortuno@ after talking offline. Repro Steps: 1. Image a device with the newest M62 test image. 2. Follow the Test DMS https://docs.google.com/document/d/1HiSuCNESc6tOQxx2QdmYATy4mT_e1zUaocmqUvJyWeU/edit# 3. Enable policy "ExtensionAllowedTypes" with values "extension", "theme", "user_script", "hosted_app", "legacy_packaged_app", "platform_app" 4. Try to launch a web shortcut from the shelf (add one via chrome browser -> hamburger menu -> More tools -> "Add to shelf..." Logs revealed that installing a web-shortcut extension with ExtensionService::OnExtensionInstalled would get extensions::disable_reason::DISABLE_REASON_LAST upon install which seems broken.
,
Nov 17 2017
,
Nov 17 2017
Wait, the extensions are actually becoming *disabled* when launched from the shelf? What about launching bookmarked apps from chrome://apps instead of using the shelf?
,
Nov 17 2017
I can't see how this isn't a relesae blocker.
,
Nov 18 2017
calamity, any chance this is related to recent PWA work?
,
Nov 18 2017
hrm, I can't repro this with a managedchrome.com account on ToT Chrome OS-on-Linux. BTW, when I set the ExtensionAllowedTypes to the values in #13 (all the checkboxes checked), then ExtensionAllowedTypes doesn't show up in chrome://policy. Do you have other policies set?
,
Nov 18 2017
"Extensions: Change how ManagementPolicy::UserMayLoad is handled" - crrev.com/c/611662 looks possibly relevant. Still can't repro though. The DISABLE_BLOCKED_BY_POLICY disable_reason was added, so it took on the new value of DISABLE_REASON_LAST. This is persisted in extension prefs. But that would only matter if you're using prefs from a newer version on an older build, right? Another wild theory: are bookmark apps even supposed to be blockable? (Why would "Add to shelf" show up in the Chrome menu then?) If not, maybe some special case for bookmark apps no longer works due to extensions being disabled by policy?
,
Nov 18 2017
As per https://bugs.chromium.org/p/chromium/issues/detail?id=785331#c3, this happens when ExtensionInstallBlacklist is set to block everything. newcomer@: Did you have this policy set as well while reproducing this? If that's the case, as c#19 points out crrev.com/c/611662 might be relevant and possibly some special case for bookmark apps no longer works. Would it be possible to repro this on a Linux workstation using 'target_os="chromeos"? I don't have access to a ChromeOS device.
,
Nov 20 2017
chrome://apps is disabled on ChromeOS
I can't get the behavior to repro on ToT with just "hosted_apps" enabled in policy. If I remove it from the allowed types in policy, then it takes the behavior described here (which is more or less expected since hosted apps are disabled and bookmark apps are hosted apps).
I don't think recent PWA work would have affected this. 2 milestones ago is before most of the overhauling work.
FYI for repro, I'm using the magic ChromeOS policy file for linux chromeos testing that you can write to at /etc/chromium/policies/managed/test_policy.json with a value of
{
"ExtensionAllowedTypes": [
"extension",
"theme",
"user_script",
"legacy_packaged_app",
"platform_app"
]
}
If we want a special case for bookmark apps and PWAs here, we should add extra allowed types so that it's explicit.
,
Nov 20 2017
calamity@: The CrOS bookmark apps being talked about in this bug are the same as the bookmark apps on on other Desktop platforms which appear on chrome://apps, correct?
,
Nov 20 2017
Yeah, that's right. If this turns out to be because the blacklist is on, I'm not sure what the right way forward is. The way forward will depend on @newcomer's response, since I can't get this to repro with only the hosted_app allow policy enabled.
,
Nov 20 2017
So I also wasn't able to repro this on Linux with ExtensionAllowedTypes set to hosted_app. newcomer@: what do you see on chrome://policy? However, I was able to repro this with ExtensionInstallBlacklist policy set to block all extensions ( crbug.com/785331 ) (Which is probably WAI since bookmark apps are a kind of extension). Also I investigated and this behavior changed after crrev.com/c/611662 which made the ManagementPolicy checks more consistent. Here's what happended for bookmark apps before crrev.com/c/611662 with ExtensionInstallBlacklist set to "*". On launching the browser, - InstalledLoader::Load would fail the ManagementPolicy::UserMayLoad check causing the bookmark app to not be enabled. - But later ExtensionSyncService::ApplyBookmarkAppSyncData would lead to a call to CrxInstaller::InstallWebApp causing the bookmark app to be installed/enabled. This code path didn't check ManagementPolicy::UserMayLoad, which was a bug. After crrev.com/c/611662, the bookmark app would failed the ManagementPolicy check in InstalledLoader and get disabled (earlier it was not loaded). Hence now during the ExtensionSyncService::ApplySyncData code path, the extension would be recognised as being disabled and we won't try to enable it again. So two questions: - Do we want to special case bookmark apps so that they can't be blacklisted (with ExtensionInstalledBlacklist/ExtensionAllowedTypes and possibly other policies)? - Are bookmark apps not working with ExtensionAllowedTypes set to "hosted_app". This would be a bug. Also I haven't yet looked into the disable reason being extensions::disable_reason::DISABLE_REASON_LAST which seems wrong.
,
Nov 20 2017
So, how do "bookmark apps" differ from regular web pages? I'm presuming that bookmarks apps are bookmarks, and so should be managed with bookmark policies (i.e. EditBookmarksEnabled, URLBlacklist) not with extension policy. My suggestion is: Yes, we want to special case "bookmark apps" so they can't be blacklisted by extension policy. I don't think we'd treat them as hosted apps, because unlike hosted apps, you don't pre-grant permissions to bookmarks. Again, apologies if "bookmark apps" is a real thing with pre-granted permission, etc, rather than just regular ol' bookmarks pinned to the shelf with maybe some extra window dressing and I didn't get the memo.
,
Nov 20 2017
I agree with #25 that enterprise policy for apps/extensions probably shouldn't apply to bookmark apps. But bookmark apps are not bookmarks, they're actually apps. Indeed, they don't grant permissions; they do nothing but afford an icon in the shelf and launcher. Whether we should use the policy for hosted apps (bookmark apps are a permissionless subset of hosted apps), or bookmarks, or no policy at all depends on what administrators will want to enforce: * Are admins interested in restricting the permissions/capabilities users can grant to apps/websites? Allow restricting hosted apps, but don't apply that policy to bookmark apps. * Are admins interested in preventing users from adding icons to the launcher or the shelf? Allow restricting apps in a way that covers bookmark apps. The second point there is that bookmark apps look and feel sort of like apps (they have icons and are uninstallable); will admins want to restrict that UX for some reason? (Is there another policy that already does this?) Technobabble follows: Adding a website to the shelf creates a bookmark app. This is a skeleton app that Chrome generates on-demand in response to that action. The app is created in such a way that launching it does nothing but open the given URL. The app has an icon in the launcher (or on chrome://apps outside Chrome OS) and can be pinned to the shelf, but has zero other "app" properties from a UX perspective AFAIK. The fact that we implement it as an app is a technical/historical detail, I think -- it's a way to get the icon into the launcher and the shelf. More clarifications available at https://chromium.googlesource.com/chromium/src/+/master/extensions/docs/extension_and_app_types.md.
,
Nov 20 2017
Yeah, I looked at it a bit more deeply after writing the comment, and "bookmark apps" indeed are not bookmarks (cue Dr. Nick). So ignore everything I wrote before except for the piece that "bookmark apps should not be controlled by extension policy" :) We've had no customer requests to block pinning web pages to the shelf, so for now let's just leave this functionality unmanaged.
,
Nov 20 2017
It's true that right now Bookmark Apps have no other "app" properties but that is changing with Desktop PWAs. Desktop PWAs will be able to capture links, just like Android apps do, and have a nicer window frame than regular Hosted Apps. It's quite possible that Desktop PWAs will be auto granted permissions in the future, the same way we auto grant some permissions to PWAs on mobile e.g. extra storage, keyboard lock, etc. This is up in the air, but I wouldn't rule it out.
,
Nov 20 2017
We're talking about bookmark apps, though, not PWAs -- or are PWAs implemented as bookmark apps?
,
Nov 20 2017
Correct. Desktop PWAs are implemented as Bookmark Apps.
,
Nov 20 2017
If pinning a PWA to the shelf provides extra capabilities for the app beyond a nicer window frame, then we may need to enable some management. Otherwise, if their capabilities are identical regardless of whether they are pinned to the shelf or opened in a browser tab, I don't see any reason why we'd add extra management (if admins don't want users to grant permissions to various origins, then they should control this via content settings the same as other web pages).
,
Nov 20 2017
This issue is marked as a release blocker with no milestone associated. Please add an appropriate milestone. All release blocking issues should have milestones associated to it, so that the issue can tracked and the fixes can be pushed promptly. Thanks for your time! To disable nags, add the Disable-Nags label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Nov 20 2017
Re #18: - I would suggest repro-ing it on a device. - If you're setting ExtensionAllowedTypes and it isn't showing up in chrome://policy, you are probably doing something incorrect, right? This should show up in chrome://policy. Re #20: - ExtensionInstallBlacklist was not set on the policy that I used to repro the bug, only the policy set in the repro steps. - I would recommend reproing this on a device after seeing others having trouble reproing this on linux. Re #21: - I'm not sure how valid this is because you're repro-ing on linux chromeos. - I reproed this when I enabled all types of apps, but this was done on device not on the emulator. re #24: - When I enabled all of the extensions, I saw all of them in chrome://policy. - I didn't have "ExtensionInstallBlacklist" set, I'm not sure how that policy is related. I'm a bit concerned that some of the discussion is based on reproing using a different policy/on the emulator. I don't have much knowledge on the policy settings, but are we sure we are barking up the right tree? Those that have been un-able to repro on-device: - Was it because it's just easier to attempt to repro on the emulator, or that the on-device repro steps didnt work for you? Setting the proper milestone for the RBS.
,
Nov 20 2017
>>I'm a bit concerned that some of the discussion is based on reproing using a different policy/on the emulator. I don't have much knowledge on the policy settings, but are we sure we are barking up the right tree? In any case, that's also something we want to solve since the behavior of bookmark apps changed after crrev.com/c/611662 and there have been bug reports for the same. ( crbug.com/785331 ) I don't have access to a Chrome OS Device currently. In the meanwhile, I'll work on a patch to special case bookmark apps for ExtensionInstallBlacklist and ExtensionAllowedTypes. Later will see if if helps the repro case that newcomer@ is experiencing (When ExtensionAllowedTypes is set to allow hosted apps).
,
Nov 21 2017
#33: It's hard to debug and dive in without a repro on my dev machine. Also, although I have a CrOS device, I am finding the instructions in that doc hard to follow. (e.g I can't open VT2 on my ChromeOS device, looked it up, needed to set to Dev mode. Now I'm stuck with the make_dev_ssd.sh step, which just fails?? Oh, I need debugging features enabled. I enabled it but didn't set a root password and it said it would be the default, but I don't know what that is?! AGGHH. Give up.). I am surprised that the policy is executing differently on the emulator vs a device because once the policy is picked up by CrOS, all the logic for what turns on/off should be the same. We are trying to distill possible causes for your behavior, which is why the ExtensionInstallBlacklist was brought up. If ExtensionInstallBlacklist is set to *, your behavior reproduces (on the emulator), so we were wondering if that was the cause. Since it's not, we investigate onwards!
,
Nov 21 2017
,
Nov 21 2017
#35: the default password is test0000 ;-) The behavior *should* be the same on Chrome OS-on-Linux with the right policy setup (using managedchrome.com, a local policy server, or writing to /etc/chromium/policies/managed/test_policy.json). It's not an "emulator" -- it's the same chrome you get on a device, just with some system services stubbed out. Policy should work mostly the same AIUI.
,
Nov 21 2017
Per #24, seems we have a way forward for at least some of the issues here. So we have several different reports: * Setting ExtensionAllowedTypes to ['hosted_app'] prevents bookmark apps from opening (#1) This is weird because bookmark apps *are* of type hosted_app. But this behavior could not be verified by #21 or #24. * Enabling all extension types in ExtensionAllowedTypes prevents bookmark apps from opening from the shelf, causes launching bookmark apps from the launcher to crash Chrome (!) (#6), and sets the extension's disable reason to DISABLE_REASON_LAST (!) (#13) The policy is used in ExtensionManagement::IsAllowedManifestType: https://cs.chromium.org/chromium/src/chrome/browser/extensions/extension_management.cc?type=cs&q=%22allowed_types%22+file:extension_management&l=159 But again, bookmark apps are TYPE_HOSTED_APP, so all indicators are that this policy should not prevent loading bookmark extensions: https://cs.chromium.org/chromium/src/chrome/browser/extensions/standard_management_policy_provider.cc?type=cs&l=99 I suspect the underlying issue is elsewhere. Need to get a stack trace from launching an item from the shelf and launcher. * Setting ExtensionInstallBlacklist to ['*'] prevents bookmark apps from opening ( issue 785331 ). Reported in 62 and 63. This makes sense given that bookmark apps are extensions. #24 and #34 suggest a fix. Note: ExtensionInstallBlacklist and ExtensionAllowedTypes are supported on linux/mac/win, so it's possible this issue repros on non-CrOS chrome as well. BTW, the difficulty of troubleshooting policy stuff is tracked in issue 752709.
,
Nov 21 2017
#35: +1 on the difficulty of following the policy troubleshooting instructions! Glad that's being tracked somewhere.
,
Nov 21 2017
#38: Yeah I was able to repro it in non-CrOS (Linux). Have a fix for whitelisting bookmark apps for ExtensionAllowedTypes, ExtensionInstallBlacklist and ExtensionSettings policies under review - https://chromium-review.googlesource.com/c/chromium/src/+/780205. Also for ChromeOS folks: App icons which correspond to disabled apps in CrOS launcher/shell should either be shown disabled (believe chrome://apps shows them as greyed out) or they should not be shown. The fact that there is no visual indication that the app is disabled and nothing happens when you click a disabled app seems like a bug.
,
Nov 21 2017
,
Nov 21 2017
#40: We have logic for that here: https://cs.chromium.org/chromium/src/chrome/browser/extensions/chrome_app_icon.cc?q=chrome_app_icon&sq=package:chromium&l=116 I'll file a speculative bug (787586) until we are sure they are not acting properly after your patch. Blocking on this one.
,
Nov 22 2017
Any updates? M63 goes stable in < 2 weeks and this is marked as a blocker.
,
Nov 23 2017
The change https://chromium-review.googlesource.com/c/chromium/src/+/780205 is still under review. I am away till Monday, so would not probably be able to get to it till then.
,
Nov 24 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/96b044cff4cbc1b6bd30bfa387b3f55d07079814 commit 96b044cff4cbc1b6bd30bfa387b3f55d07079814 Author: Christopher Lam <calamity@chromium.org> Date: Fri Nov 24 03:29:46 2017 Extensions: Whitelist bookmark apps in StandardManagementPolicyProvider. r496521 made the extension management policy checks more consistent. This had a side effect of disabling bookmark apps when the appropriate ExtensionInstallBlacklist/ExtensionAllowedTypes/ExtensionSettings policies were set. This CL whitelists bookmark apps in StandardManagementPolicyProvider::UserMayLoad. Hence the bookmark apps should no longer get disabled by the ExtensionInstallBlacklist, ExtensionAllowedTypes and ExtensionSettings policies. Based on https://chromium-review.googlesource.com/#/c/780205/ TBR=rdevlin.cronin@chromium.org BUG= 786061 TEST=On Linux, navigate to google.com. Use chrome -> settings -> more tools -> "Add to Desktop..." to create a desktop shortcut for google.com. Change ExtensionInstallBlacklist to block all extensions ("*"). Ensure the same from chrome: //policy. Click on the google.com shortcut created earlier (from chrome: //apps or Desktop) and ensure that it works correctly. Change-Id: Ic2ac1ddd2dfef6c5442eb7b0d366eaf66443c51d Reviewed-on: https://chromium-review.googlesource.com/787011 Reviewed-by: calamity <calamity@chromium.org> Reviewed-by: Maksim Ivanov <emaxx@chromium.org> Commit-Queue: calamity <calamity@chromium.org> Cr-Commit-Position: refs/heads/master@{#519052} [modify] https://crrev.com/96b044cff4cbc1b6bd30bfa387b3f55d07079814/chrome/browser/extensions/standard_management_policy_provider.cc [modify] https://crrev.com/96b044cff4cbc1b6bd30bfa387b3f55d07079814/chrome/browser/policy/policy_browsertest.cc
,
Nov 24 2017
,
Nov 24 2017
This bug requires manual review: We are only 10 days from stable. Please contact the milestone owner if you have questions. Owners: cmasso@(Android), cmasso@(iOS), gkihumba@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Nov 28 2017
Issue 775359 has been merged into this issue.
,
Nov 28 2017
Waiting for this to be validated on tot before merging into M63
,
Nov 28 2017
I was able to verify the fix on ToT on ChromeOs-on-Linux. Bookmark apps should now be exempted from extension blacklist policy checks.
,
Nov 28 2017
Talked to gkihumba@: Still needs TE verification.
,
Nov 28 2017
,
Nov 28 2017
Thanks everyone!
,
Nov 28 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fc4a5c30819b653750594eda95f334dab490ad0e commit fc4a5c30819b653750594eda95f334dab490ad0e Author: Christopher Lam <calamity@chromium.org> Date: Tue Nov 28 23:55:11 2017 Merge M63: Extensions: Whitelist bookmark apps in StandardManagementPolicyProvider. Original description: >r496521 made the extension management policy checks more consistent. This had a >side effect of disabling bookmark apps when the appropriate >ExtensionInstallBlacklist/ExtensionAllowedTypes/ExtensionSettings policies were >set. This CL whitelists bookmark apps in >StandardManagementPolicyProvider::UserMayLoad. Hence the bookmark apps should no >longer get disabled by the ExtensionInstallBlacklist, ExtensionAllowedTypes and >ExtensionSettings policies. > >Based on https://chromium-review.googlesource.com/#/c/780205/ > >TBR=rdevlin.cronin@chromium.org >BUG= 786061 >TEST=On Linux, navigate to google.com. Use chrome -> settings -> more tools -> >"Add to Desktop..." to create a desktop shortcut for google.com. Change >ExtensionInstallBlacklist to block all extensions ("*"). Ensure the same from > >chrome: //policy. Click on the google.com shortcut created earlier (from >chrome: //apps or Desktop) and ensure that it works correctly. >Change-Id: Ic2ac1ddd2dfef6c5442eb7b0d366eaf66443c51d >Reviewed-on: https://chromium-review.googlesource.com/787011 >Reviewed-by: calamity <calamity@chromium.org> >Reviewed-by: Maksim Ivanov <emaxx@chromium.org> >Commit-Queue: calamity <calamity@chromium.org> >Cr-Commit-Position: refs/heads/master@{#519052} TBR=calamity@chromium.org Change-Id: Ic2ac1ddd2dfef6c5442eb7b0d366eaf66443c51d Reviewed-on: https://chromium-review.googlesource.com/794880 Reviewed-by: Karan Bhatia <karandeepb@chromium.org> Cr-Commit-Position: refs/branch-heads/3239@{#593} Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578} [modify] https://crrev.com/fc4a5c30819b653750594eda95f334dab490ad0e/chrome/browser/extensions/standard_management_policy_provider.cc [modify] https://crrev.com/fc4a5c30819b653750594eda95f334dab490ad0e/chrome/browser/policy/policy_browsertest.cc
,
Nov 30 2017
Enterprise test team was able to repro this in M62 only with ExtensionInstallBlacklist set to "*" and ExtensionAllowedTypes set to "hosted_app", the pinned shelf icon flashes, but does nothing. Verified fixed in M63. The bookmark opens from shelf if ExtensionInstallBlacklist set to "*". Chrome: 63.0.3239.73 Chrome OS: 10032.60.0
,
Jan 23 2018
Hi team, Do we have any updates in this matter? Thanks in advance.
,
Jan 23 2018
#55 says it's fixed in M63. What other updates do need? |
||||||||||||||||||||||||
►
Sign in to add a comment |
||||||||||||||||||||||||
Comment 1 by newcomer@chromium.org
, Nov 16 2017