New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 847429 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 4
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

ChromeOS issue: bouncing flag in network settings

Project Member Reported by marcore@chromium.org, May 29 2018

Issue description

ChromeOS version: 66.0.3359.181 
ChromeOS device model: Acer Chromebox CXI2
Case#: 15925237

Description: when trying to disable the "Configure IP address automatically" and "Name servers" if in the policy DeviceOpenNetworkConfiguration there is the configuration for the same network, the switch is re-enabled automatically 


Steps to reproduce: 
configure this policy:
"DeviceOpenNetworkConfiguration": {
         "level": "mandatory",
         "scope": "machine",
         "source": "sourceCloud",
         "value": "{\n   \"GlobalNetworkConfiguration\": {\n      \"AllowOnlyPolicyNetworksToAutoconnect\": true,\n      \"AllowOnlyPolicyNetworksToConnect\": false\n   },\n   \"NetworkConfigurations\": [ {\n      \"GUID\": \"{TEST-69aa-4206-9423-3c68c29b31ac}\",\n      \"Name\": \"TESTWIFI\",\n      \"ProxySettings\": {\n         \"Type\": \"Direct\"\n      },\n      \"Type\": \"WiFi\",\n      \"WiFi\": {\n         \"AutoConnect\": false,\n         \"HiddenSSID\": false,\n         \"SSID\": \"TESTWIFI\",\n         \"Security\": \"None\"\n      }\n   } ]\n}\n"
      },
go to chrome://settings/networkDetail?guid=%7BTEST-69aa-4206-9423-3c68c29b31ac%7D&type=WiFi&name=TESTWIFI in the network section


Current Behavior / Reproduction: 
it's shown a enable/disable switch but it's automatically always on
Expected Behavior: 
display the icon and string "This setting is enforced by your administrator" and disable the switch
Drive link to logs: 
policy: https://drive.google.com/open?id=1QH3cKj1eFTjV8R2_nNEEB_VjyfPUZGrp
video of the customer: https://drive.google.com/open?id=1gly9HsgJjOrfkDGdLTBAkC43-2cLLCbQ
 
Cc: steve...@chromium.org
Steven, can you help ?

Comment 2 by jayhlee@google.com, May 29 2018

I'm confused by this description.

  Where is customer setting policy? Admin console? ChromeAD?

  What is customer trying to do here? What is expected behavior vs actual behavior?
the configuration is made on the Admin Console
the customer is trying to disable the DHCP for a wifi network configured/pushed in the policy.
in the video it's shown what is happening, you try to disable, for some millisecond it's shown the input field, then it's back enabled
I've tried with some very old version(M55),(before the change of the UI) and in that version it was "This setting is enforced by your administrator" 
Labels: -OS-Chrome -M-66 M-68
Owner: steve...@chromium.org
Status: Assigned (was: Untriaged)
This doesn't seem urgent, but I can try to fix it for 68.

Labels: -M-68 OS-Chrome
Labels: M-70
We have another affected by this issue, is there any progress on it? 
Or at least some insight what's the expected behavior here? I have a suspicion that this setting would be gray (unchangeable) on device end, am I wrong? Answer would allow to set customer expectations right.
BTW, same setting works for user-level WiFi networks.
Cc: vkhabarov@chromium.org
Cc: hendrich@chromium.org
There are actually two separate issues here:
1. For a number of reasons it may be the case that a change in the UI may not get reflected in Shill, particularly around IP and Nameserver values.
2. We may not be correctly indicating all properties that are enforced by policy.

(1) is challenging to resolve, which is why this is still on the backburner.
(2) may be an oversight and can be addressed. We have some similar, more urgent cases where we need policy indicators.

+hendrich@ re (2)

re (2) Yes, this is on my radar and will probably be tackled by one of our nooglers joining in October along with crbug.com/869199
Labels: Hotlist-GoodFirstBug
Owner: hendrich@chromium.org
Owner: voit@google.com
@stevenjb Regarding (1):

If admin has specified ip config via policy like this:
{
   "IPAddressConfigType": "DHCP",
   "Recommended": ["IPAddressConfigType"]
}

If user tries to change the ip config type to static later and sets config to something like this:
{
   "IPAddressConfigType": "Static",
   "StaticIPConfig": {
      "Gateway": "100.113.47.254",
      "IPAddress": "100.113.42.76",
      "RoutingPrefix": 21,
      "Type": "IPv4"
   }
}

Here the "IPAddressConfigType" is recommended and it will overridden by user setting just fine. But user can't override the "StaticIPConfig" because device policy does not have this field. Also admin can't set StaticIPConfig as recommended because it's of object type.

This probably is challenging to merge settings in a way that lets user change the ip config correctly in this case.

But we can relatively easily just disable ip address/namespaces settings UI for policy-managed network.

Maybe you have an idea how should we handle cases of some fields missing in policy value to let user override these settings? Or I should just go with disabling the UI?
Currently, IIRC, the admin would need to provide a StaticIPConfig object and set each StaticIPConfig field value to Recommended. Those should be ignored if the config type is DHCP.

We could probably add some code to treat all static ip config properties as recommended if IPAddressConfigType is recommended.



Maybe it's better to let users use dictionary values in the "Recommended" list and in onc_merger just set user_editable flag to true for each child of such recommended dictionary?

For example:

Admin config:
{
  "IPAddressConfigType": "DHCP",
  "Recommended": ["IPAddressConfigType", "StaticIPConfig"]
}

User config:
{
  "IPAddressConfigType": "Static",
  "StaticIPConfig": {
    "Gateway": "100.113.47.254",
    "IPAddress": "100.113.42.76",
    "RoutingPrefix": 21,
    "Type": "IPv4"
  }
}

Would result in user config correctly applied because every field would be user-editable. It's seems to be much cleaner approach than filling StaticIPConfig with recommended values (at least because StaticIPConfig.Type is a required field and we will need to specify it even if it's recommended).

What do you think?
That sounds reasonable to me and wouldn't require special casing any other potential examples with a similar pattern. It should also make for better UI which will just show the enterprise icon for the config type and not for the individual properties (which do not have actual recommended values).

Hi,

Apologies but can we request an update for this bug?

Thanks.
Status: Started (was: Assigned)
Hi, this bug is going to be fixed in dev this week
Project Member

Comment 18 by bugdroid1@chromium.org, Dec 4

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e9d087a00c99e2eb1d2654133700dd6b12d15b7a

commit e9d087a00c99e2eb1d2654133700dd6b12d15b7a
Author: Zakhar Voit <voit@google.com>
Date: Tue Dec 04 03:22:01 2018

Allow user to modify network settings not enforced by device policy.

* ONC normalizer now removes StaticIPConfig fields that are
overriden by IPAddressConfigType and NameServersConfigType.

* ONC validator validates StaticIPConfig properly (e.g. requires
fields based on IPAddressConfigType and NameServersConfigType)

Bug:  847429 
Change-Id: I47fceaaf10f41346afba552a35206fcdb130a887
Reviewed-on: https://chromium-review.googlesource.com/c/1288576
Reviewed-by: Alexander Hendrich <hendrich@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: Zakhar Voit <voit@google.com>
Cr-Commit-Position: refs/heads/master@{#613409}
[modify] https://crrev.com/e9d087a00c99e2eb1d2654133700dd6b12d15b7a/chrome/browser/extensions/api/networking_private/networking_private_chromeos_apitest.cc
[modify] https://crrev.com/e9d087a00c99e2eb1d2654133700dd6b12d15b7a/chrome/test/data/extensions/api_test/networking_private/chromeos/test.js
[modify] https://crrev.com/e9d087a00c99e2eb1d2654133700dd6b12d15b7a/chrome/test/data/extensions/api_test/networking_private/service_client/test.js
[modify] https://crrev.com/e9d087a00c99e2eb1d2654133700dd6b12d15b7a/chromeos/network/onc/onc_normalizer.cc
[modify] https://crrev.com/e9d087a00c99e2eb1d2654133700dd6b12d15b7a/chromeos/network/onc/onc_normalizer.h
[modify] https://crrev.com/e9d087a00c99e2eb1d2654133700dd6b12d15b7a/chromeos/network/onc/onc_normalizer_unittest.cc
[modify] https://crrev.com/e9d087a00c99e2eb1d2654133700dd6b12d15b7a/chromeos/network/onc/onc_signature.cc
[modify] https://crrev.com/e9d087a00c99e2eb1d2654133700dd6b12d15b7a/chromeos/network/onc/onc_validator.cc
[modify] https://crrev.com/e9d087a00c99e2eb1d2654133700dd6b12d15b7a/chromeos/network/onc/onc_validator.h
[modify] https://crrev.com/e9d087a00c99e2eb1d2654133700dd6b12d15b7a/chromeos/test/data/network/invalid_settings_with_repairs.json
[modify] https://crrev.com/e9d087a00c99e2eb1d2654133700dd6b12d15b7a/chromeos/test/data/network/settings_with_normalization.json
[modify] https://crrev.com/e9d087a00c99e2eb1d2654133700dd6b12d15b7a/components/onc/docs/onc_spec.md

Project Member

Comment 19 by bugdroid1@chromium.org, Jan 4

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6d16ec710f1023c07517d0942fc967f30574c905

commit 6d16ec710f1023c07517d0942fc967f30574c905
Author: Zakhar Voit <voit@google.com>
Date: Fri Jan 04 12:58:10 2019

Fix incorrect behaviour of settings UI for IP address and name servers.

* Block IP address and name servers checkboxes and input fields when
corresponding settings are enforced by policy and not recommended.
* Correctly show policy indicators for these settings.

Bug:  847429 
Change-Id: I3db8ad751ebe44dff487cc01721019738f72c4e5
Reviewed-on: https://chromium-review.googlesource.com/c/1335487
Commit-Queue: Zakhar Voit <voit@google.com>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Alexander Hendrich <hendrich@chromium.org>
Cr-Commit-Position: refs/heads/master@{#619929}
[modify] https://crrev.com/6d16ec710f1023c07517d0942fc967f30574c905/ui/webui/resources/cr_components/chromeos/network/network_ip_config.html
[modify] https://crrev.com/6d16ec710f1023c07517d0942fc967f30574c905/ui/webui/resources/cr_components/chromeos/network/network_ip_config.js
[modify] https://crrev.com/6d16ec710f1023c07517d0942fc967f30574c905/ui/webui/resources/cr_components/chromeos/network/network_nameservers.html
[modify] https://crrev.com/6d16ec710f1023c07517d0942fc967f30574c905/ui/webui/resources/cr_components/chromeos/network/network_nameservers.js
[modify] https://crrev.com/6d16ec710f1023c07517d0942fc967f30574c905/ui/webui/resources/cr_elements/policy/cr_policy_network_behavior.js

Project Member

Comment 20 by bugdroid1@chromium.org, Jan 4

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/873fae304dc79446fe371efe75ca1bfe824c0bab

commit 873fae304dc79446fe371efe75ca1bfe824c0bab
Author: Xida Chen <xidachen@chromium.org>
Date: Fri Jan 04 14:55:06 2019

Revert "Fix incorrect behaviour of settings UI for IP address and name servers."

This reverts commit 6d16ec710f1023c07517d0942fc967f30574c905.

Reason for revert: Suspect causing failure to the net_unittests here:
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Mac10.10%20Tests/38075

Original change's description:
> Fix incorrect behaviour of settings UI for IP address and name servers.
> 
> * Block IP address and name servers checkboxes and input fields when
> corresponding settings are enforced by policy and not recommended.
> * Correctly show policy indicators for these settings.
> 
> Bug:  847429 
> Change-Id: I3db8ad751ebe44dff487cc01721019738f72c4e5
> Reviewed-on: https://chromium-review.googlesource.com/c/1335487
> Commit-Queue: Zakhar Voit <voit@google.com>
> Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
> Reviewed-by: Alexander Hendrich <hendrich@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#619929}

TBR=stevenjb@chromium.org,hendrich@chromium.org,voit@google.com

Change-Id: I986daf88eb60ac7e1e5113ef9c857b473a7f8d0e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  847429 
Reviewed-on: https://chromium-review.googlesource.com/c/1396219
Reviewed-by: Xida Chen <xidachen@chromium.org>
Commit-Queue: Xida Chen <xidachen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#619947}
[modify] https://crrev.com/873fae304dc79446fe371efe75ca1bfe824c0bab/ui/webui/resources/cr_components/chromeos/network/network_ip_config.html
[modify] https://crrev.com/873fae304dc79446fe371efe75ca1bfe824c0bab/ui/webui/resources/cr_components/chromeos/network/network_ip_config.js
[modify] https://crrev.com/873fae304dc79446fe371efe75ca1bfe824c0bab/ui/webui/resources/cr_components/chromeos/network/network_nameservers.html
[modify] https://crrev.com/873fae304dc79446fe371efe75ca1bfe824c0bab/ui/webui/resources/cr_components/chromeos/network/network_nameservers.js
[modify] https://crrev.com/873fae304dc79446fe371efe75ca1bfe824c0bab/ui/webui/resources/cr_elements/policy/cr_policy_network_behavior.js

Project Member

Comment 21 by bugdroid1@chromium.org, Jan 4

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f10a0ce04b1210867cf9b27b68b4c0bab155a3a6

commit f10a0ce04b1210867cf9b27b68b4c0bab155a3a6
Author: Xida Chen <xidachen@chromium.org>
Date: Fri Jan 04 15:16:07 2019

Reland "Fix incorrect behaviour of settings UI for IP address and name servers."

This reverts commit 873fae304dc79446fe371efe75ca1bfe824c0bab.

Reason for revert:
This is not the cause of the net_unittests failure, apparent there is crbug.com/869227
Sorry about that.

Original change's description:
> Revert "Fix incorrect behaviour of settings UI for IP address and name servers."
> 
> This reverts commit 6d16ec710f1023c07517d0942fc967f30574c905.
> 
> Reason for revert: Suspect causing failure to the net_unittests here:
> https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Mac10.10%20Tests/38075
> 
> Original change's description:
> > Fix incorrect behaviour of settings UI for IP address and name servers.
> > 
> > * Block IP address and name servers checkboxes and input fields when
> > corresponding settings are enforced by policy and not recommended.
> > * Correctly show policy indicators for these settings.
> > 
> > Bug:  847429 
> > Change-Id: I3db8ad751ebe44dff487cc01721019738f72c4e5
> > Reviewed-on: https://chromium-review.googlesource.com/c/1335487
> > Commit-Queue: Zakhar Voit <voit@google.com>
> > Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
> > Reviewed-by: Alexander Hendrich <hendrich@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#619929}
> 
> TBR=stevenjb@chromium.org,hendrich@chromium.org,voit@google.com
> 
> Change-Id: I986daf88eb60ac7e1e5113ef9c857b473a7f8d0e
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug:  847429 
> Reviewed-on: https://chromium-review.googlesource.com/c/1396219
> Reviewed-by: Xida Chen <xidachen@chromium.org>
> Commit-Queue: Xida Chen <xidachen@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#619947}

TBR=stevenjb@chromium.org,xidachen@chromium.org,hendrich@chromium.org,voit@google.com

Change-Id: I8e7a192ea6ea63f18d520212ef3792864d2c1068
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  847429 
Reviewed-on: https://chromium-review.googlesource.com/c/1396224
Reviewed-by: Xida Chen <xidachen@chromium.org>
Commit-Queue: Xida Chen <xidachen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#619953}
[modify] https://crrev.com/f10a0ce04b1210867cf9b27b68b4c0bab155a3a6/ui/webui/resources/cr_components/chromeos/network/network_ip_config.html
[modify] https://crrev.com/f10a0ce04b1210867cf9b27b68b4c0bab155a3a6/ui/webui/resources/cr_components/chromeos/network/network_ip_config.js
[modify] https://crrev.com/f10a0ce04b1210867cf9b27b68b4c0bab155a3a6/ui/webui/resources/cr_components/chromeos/network/network_nameservers.html
[modify] https://crrev.com/f10a0ce04b1210867cf9b27b68b4c0bab155a3a6/ui/webui/resources/cr_components/chromeos/network/network_nameservers.js
[modify] https://crrev.com/f10a0ce04b1210867cf9b27b68b4c0bab155a3a6/ui/webui/resources/cr_elements/policy/cr_policy_network_behavior.js

Status: Fixed (was: Started)

Sign in to add a comment