Issue metadata
Sign in to add a comment
|
Able to change locked wallpaper
Reported by
tsch...@lisle202.org,
Jul 27 2017
|
|||||||||||||||||||||||||||
Issue descriptionUserAgent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/59.0.3071.115 Safari/537.36 Platform: 9460.73.0 Steps to reproduce the problem: 1. Wallpaper locked in admin console 2. On managed CB with wallpaper locked, settings > wallpaper app 3. Able to change background to any image What is the expected behavior? Should not be able to change background What went wrong? CB background should be locked for student devices and this allows students to change their wallpaper. Did this work before? Yes May 2017 version (don't know exact) Chrome version: 59.0.3071.134 Channel: stable OS Version: 59 Flash Version:
,
Jul 28 2017
I'll take a look at it.
,
Aug 2 2017
It's a regression caused by the new MD-settings page. There is no guard against wallpaper policy users any more. I'll work on a fix.
,
Aug 2 2017
,
Aug 4 2017
Students in my Google Apps domain are also able to change their background wallpaper to any image
,
Aug 4 2017
,
Aug 18 2017
Students in my Google Apps domain also have the ability to change their background wallpaper to any image they would like.
,
Aug 22 2017
Hello, Just wondering if there is an update on a possible fix for this yet? Thank you!
,
Aug 25 2017
Sorry I haven't had time to fix it yet. Will try to work on it next week. +Steven since it's a Md setting regression.
,
Aug 25 2017
We shouldn't be relying on the UI for policy enforcement. While we should certainly disable the control in Settings if appropriate, we should also block the change request in the model.
,
Sep 7 2017
Talked with minch@. She has cycle to implement it. I'll assist her to fix this asap.
,
Sep 11 2017
Watching Case
,
Sep 11 2017
,
Sep 21 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/67dc59ad81be7cb9849592739fe3ed4e32295431 commit 67dc59ad81be7cb9849592739fe3ed4e32295431 Author: MinChen <minch@chromium.org> Date: Thu Sep 21 17:22:44 2017 Make it can't open wallpaper manager if wallpaper is policy controlled. Changes, 1. Show the wallpaper setting row only for login, whitelist types and active users (should satisfy the whole conditions at the same time). 2. Only can open the wallpaper manager if the wallpaper is not policy controlled. 3. Added wallpaper policy indicator. Show it if the wallpaper is policy controlled. Bug: 749755 Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation Change-Id: Ie6b9875db7f1958d3cdd8fb9fc817e4df79782b3 Reviewed-on: https://chromium-review.googlesource.com/661070 Reviewed-by: Steven Bennetts <stevenjb@chromium.org> Reviewed-by: Xiaoqian Dai <xdai@chromium.org> Commit-Queue: min c <minch@chromium.org> Cr-Commit-Position: refs/heads/master@{#503487} [modify] https://crrev.com/67dc59ad81be7cb9849592739fe3ed4e32295431/chrome/browser/resources/settings/appearance_page/appearance_browser_proxy.js [modify] https://crrev.com/67dc59ad81be7cb9849592739fe3ed4e32295431/chrome/browser/resources/settings/appearance_page/appearance_page.html [modify] https://crrev.com/67dc59ad81be7cb9849592739fe3ed4e32295431/chrome/browser/resources/settings/appearance_page/appearance_page.js [modify] https://crrev.com/67dc59ad81be7cb9849592739fe3ed4e32295431/chrome/browser/resources/settings/page_visibility.js [modify] https://crrev.com/67dc59ad81be7cb9849592739fe3ed4e32295431/chrome/browser/ui/webui/settings/appearance_handler.cc [modify] https://crrev.com/67dc59ad81be7cb9849592739fe3ed4e32295431/chrome/browser/ui/webui/settings/appearance_handler.h [modify] https://crrev.com/67dc59ad81be7cb9849592739fe3ed4e32295431/chrome/test/data/webui/settings/appearance_page_test.js [modify] https://crrev.com/67dc59ad81be7cb9849592739fe3ed4e32295431/chrome/test/data/webui/settings/settings_main_test.js
,
Sep 21 2017
,
Sep 25 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2f0d58d03c0dcca6ae35fac2aa2c1a0e63be28d8 commit 2f0d58d03c0dcca6ae35fac2aa2c1a0e63be28d8 Author: min c <minch@chromium.org> Date: Mon Sep 25 19:55:05 2017 Revert "Make it can't open wallpaper manager if wallpaper is policy controlled." This reverts commit 67dc59ad81be7cb9849592739fe3ed4e32295431. Reason for revert: Caused issue 768319 . Will revert first and fix it followed. Original change's description: > Make it can't open wallpaper manager if wallpaper is policy controlled. > > Changes, > 1. Show the wallpaper setting row only for login, whitelist types and active users > (should satisfy the whole conditions at the same time). > 2. Only can open the wallpaper manager if the wallpaper is not policy controlled. > 3. Added wallpaper policy indicator. Show it if the wallpaper is policy > controlled. > > Bug: 749755 > Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation > Change-Id: Ie6b9875db7f1958d3cdd8fb9fc817e4df79782b3 > Reviewed-on: https://chromium-review.googlesource.com/661070 > Reviewed-by: Steven Bennetts <stevenjb@chromium.org> > Reviewed-by: Xiaoqian Dai <xdai@chromium.org> > Commit-Queue: min c <minch@chromium.org> > Cr-Commit-Position: refs/heads/master@{#503487} TBR=stevenjb@chromium.org,xdai@chromium.org,minch@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 749755 Change-Id: Ib8214a33dd0ebf8b6b690b9e15256ccaf243b452 Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation Reviewed-on: https://chromium-review.googlesource.com/682574 Reviewed-by: min c <minch@chromium.org> Commit-Queue: min c <minch@chromium.org> Cr-Commit-Position: refs/heads/master@{#504140} [modify] https://crrev.com/2f0d58d03c0dcca6ae35fac2aa2c1a0e63be28d8/chrome/browser/resources/settings/appearance_page/appearance_browser_proxy.js [modify] https://crrev.com/2f0d58d03c0dcca6ae35fac2aa2c1a0e63be28d8/chrome/browser/resources/settings/appearance_page/appearance_page.html [modify] https://crrev.com/2f0d58d03c0dcca6ae35fac2aa2c1a0e63be28d8/chrome/browser/resources/settings/appearance_page/appearance_page.js [modify] https://crrev.com/2f0d58d03c0dcca6ae35fac2aa2c1a0e63be28d8/chrome/browser/resources/settings/page_visibility.js [modify] https://crrev.com/2f0d58d03c0dcca6ae35fac2aa2c1a0e63be28d8/chrome/browser/ui/webui/settings/appearance_handler.cc [modify] https://crrev.com/2f0d58d03c0dcca6ae35fac2aa2c1a0e63be28d8/chrome/browser/ui/webui/settings/appearance_handler.h [modify] https://crrev.com/2f0d58d03c0dcca6ae35fac2aa2c1a0e63be28d8/chrome/test/data/webui/settings/appearance_page_test.js [modify] https://crrev.com/2f0d58d03c0dcca6ae35fac2aa2c1a0e63be28d8/chrome/test/data/webui/settings/settings_main_test.js
,
Sep 26 2017
Hello guys, I have a customer still reporting this issue. I wanted to know if the fix is applied to a specific version? Thank you for the assistance
,
Sep 26 2017
We fixed it in M-63, I think it haven't been released yet. Let me try to add Merge-Request-62 to see whether we can merge it to M-62. And the previous fix has been reverted yesterday, the new fix is still under review.
,
Sep 26 2017
This bug requires manual review: Reverts referenced in bugdroid comments after merge request. Please contact the milestone owner if you have questions. Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Sep 26 2017
We should land the new fix on ToT then we can review the merge, this is probably mergable once we verify ToT works.
,
Sep 28 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1c8d60574cb3aa2db0dc9bb96010aa52ec4bd055 commit 1c8d60574cb3aa2db0dc9bb96010aa52ec4bd055 Author: MinChen <minch@chromium.org> Date: Thu Sep 28 00:39:09 2017 [Reland]Make it can't open wallpaper manager if wallpaper is policy controlled. Changes, 1. Show the wallpaper setting row only for login, whitelist types and active users (should satisfy the whole conditions at the same time). 2. Only can open the wallpaper manager if the wallpaper is not policy controlled. 3. Added wallpaper policy indicator. Show it if the wallpaper is policy controlled. Bug: 749755 , 768319 Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation Change-Id: I07ffb372bf511411188b847c969cdc2341658bfa Reviewed-on: https://chromium-review.googlesource.com/683221 Reviewed-by: Steven Bennetts <stevenjb@chromium.org> Reviewed-by: Xiaoqian Dai <xdai@chromium.org> Commit-Queue: min c <minch@chromium.org> Cr-Commit-Position: refs/heads/master@{#504820} [modify] https://crrev.com/1c8d60574cb3aa2db0dc9bb96010aa52ec4bd055/chrome/browser/resources/settings/appearance_page/appearance_browser_proxy.js [modify] https://crrev.com/1c8d60574cb3aa2db0dc9bb96010aa52ec4bd055/chrome/browser/resources/settings/appearance_page/appearance_page.html [modify] https://crrev.com/1c8d60574cb3aa2db0dc9bb96010aa52ec4bd055/chrome/browser/resources/settings/appearance_page/appearance_page.js [modify] https://crrev.com/1c8d60574cb3aa2db0dc9bb96010aa52ec4bd055/chrome/browser/resources/settings/page_visibility.js [modify] https://crrev.com/1c8d60574cb3aa2db0dc9bb96010aa52ec4bd055/chrome/browser/resources/settings/settings_menu/settings_menu.html [modify] https://crrev.com/1c8d60574cb3aa2db0dc9bb96010aa52ec4bd055/chrome/browser/ui/webui/settings/appearance_handler.cc [modify] https://crrev.com/1c8d60574cb3aa2db0dc9bb96010aa52ec4bd055/chrome/browser/ui/webui/settings/appearance_handler.h [modify] https://crrev.com/1c8d60574cb3aa2db0dc9bb96010aa52ec4bd055/chrome/test/data/webui/settings/appearance_page_test.js [modify] https://crrev.com/1c8d60574cb3aa2db0dc9bb96010aa52ec4bd055/chrome/test/data/webui/settings/settings_main_test.js [modify] https://crrev.com/1c8d60574cb3aa2db0dc9bb96010aa52ec4bd055/chrome/test/data/webui/settings/settings_menu_test.js
,
Oct 11 2017
Verified fixed in M63. User is not able to change background wallpaper (see attached screenshot). Chrome: 63.0.3230.0 Platform: ChromeOS 10004.0.0 dev Device: Astronaut Need to be merged to M62.
,
Oct 11 2017
This bug requires manual review: We are only 5 days from stable. Please contact the milestone owner if you have questions. Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Oct 12 2017
Approved for 62.
,
Oct 13 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5e5407114f7169b3f14aae42b02fa8835da7604e commit 5e5407114f7169b3f14aae42b02fa8835da7604e Author: MinChen <minch@chromium.org> Date: Fri Oct 13 01:04:39 2017 [Merge to M62][Reland]Make it can't open wallpaper manager if wallpaper is policy controlled. TBR=minch@chromium.org Changes, 1. Show the wallpaper setting row only for login, whitelist types and active users (should satisfy the whole conditions at the same time). 2. Only can open the wallpaper manager if the wallpaper is not policy controlled. 3. Added wallpaper policy indicator. Show it if the wallpaper is policy controlled. (cherry picked from commit 1c8d60574cb3aa2db0dc9bb96010aa52ec4bd055) Bug: 749755 , 768319 Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation Change-Id: I07ffb372bf511411188b847c969cdc2341658bfa Reviewed-on: https://chromium-review.googlesource.com/683221 Reviewed-by: Steven Bennetts <stevenjb@chromium.org> Reviewed-by: Xiaoqian Dai <xdai@chromium.org> Commit-Queue: min c <minch@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#504820} Reviewed-on: https://chromium-review.googlesource.com/717716 Reviewed-by: min c <minch@chromium.org> Cr-Commit-Position: refs/branch-heads/3202@{#676} Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098} [modify] https://crrev.com/5e5407114f7169b3f14aae42b02fa8835da7604e/chrome/browser/resources/settings/appearance_page/appearance_browser_proxy.js [modify] https://crrev.com/5e5407114f7169b3f14aae42b02fa8835da7604e/chrome/browser/resources/settings/appearance_page/appearance_page.html [modify] https://crrev.com/5e5407114f7169b3f14aae42b02fa8835da7604e/chrome/browser/resources/settings/appearance_page/appearance_page.js [modify] https://crrev.com/5e5407114f7169b3f14aae42b02fa8835da7604e/chrome/browser/resources/settings/page_visibility.js [modify] https://crrev.com/5e5407114f7169b3f14aae42b02fa8835da7604e/chrome/browser/resources/settings/settings_menu/settings_menu.html [modify] https://crrev.com/5e5407114f7169b3f14aae42b02fa8835da7604e/chrome/browser/ui/webui/settings/appearance_handler.cc [modify] https://crrev.com/5e5407114f7169b3f14aae42b02fa8835da7604e/chrome/browser/ui/webui/settings/appearance_handler.h [modify] https://crrev.com/5e5407114f7169b3f14aae42b02fa8835da7604e/chrome/test/data/webui/settings/appearance_page_test.js [modify] https://crrev.com/5e5407114f7169b3f14aae42b02fa8835da7604e/chrome/test/data/webui/settings/settings_main_test.js [modify] https://crrev.com/5e5407114f7169b3f14aae42b02fa8835da7604e/chrome/test/data/webui/settings/settings_menu_test.js
,
Oct 13 2017
,
Oct 23 2017
Issue 715745 has been merged into this issue.
,
Nov 6 2017
,
Nov 11 2017
Reopened, because the following behavior was observed in M62: 1. Enroll device to the domain. 2. Sing in as a new user. 3. After 3 min the default restricted wallpaper turns to another image, user can change background wallpaper (see attached screenshots). 4. When user sign out and sign in again, the default restricted wallpaper is shown and not changing for existing user. 5. Only in case if user removed and added as a new one, the wallpaper image changes. Chrome: 62.0.3202.82 Chrome OS: 9901.66.0 Device: Kevin Also find attached the logs. Please note, that in M63 no such problem.
,
Nov 14 2017
Re#29: I think there is a race between Chrome sync and policy enforcement for the new user case. If Chrome sync kicks off later than policy enforcement and if the user is using a ONLINE wallpaper (selected from the pre-bundled wallpapers), the ONLINE wallpaper can overwrite the policy wallpaper, which it should not. It has been fixed in https://chromium-review.googlesource.com/c/chromium/src/+/656598 in M63. Giving 62 has been stable and 63 is going out soon, I don't think this is serious enough to merge back to 62. So close it.
,
Nov 15 2017
This is fixed in M63 (see #c22). M62 is already in stable hence has missed the M62 boat. Marking this as verified for M63. @omrilio, wdyt?
,
Nov 15 2017
Since this is a race condition and likely not a common case and was fixed in a build that's rolling out in 3 weeks, that sounds like a good plan.
,
Nov 16 2017
Feels kind of fragile to have a bunch of IsPolicyControlled() calls sprinkled through the code. Shouldn't WallpaperManager just refuse to transition from policy->non-policy wallpaper without an explicit call to reset policy first? That feels like the right place to enforce this, and would be congruent with the rest of the policy handlers elsewhere in the codebase?
,
Nov 16 2017
,
Nov 17 2017
Re#33 atwilson@: Thanks for the suggestion! The reason I put the policy check in wallpaper_private_api.cc instead of WallpaperManager is to avoid reading image file and decoding image before even trying to set it. WallpaperManager accepts the decoded image and set it as the user's wallpaper. I think We probably should move the reading/decoding image logic to WallpaperManager too and then do the policy check there.
,
Dec 29 2017
,
Jan 18 2018
Is this issue resolved?
,
Jan 18 2018
Yes, the issue was fixed in version 63.
,
Mar 9 2018
,
Mar 9 2018
|
||||||||||||||||||||||||||||
►
Sign in to add a comment |
||||||||||||||||||||||||||||
Comment 1 by rsorokin@chromium.org
, Jul 28 2017Labels: Enterprise-Triaged
Owner: x...@chromium.org