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

Issue 855057 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Sep 7
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

Surface errors from ONC validator

Project Member Reported by ljusten@chromium.org, Jun 21 2018

Issue description

We'd like to surface errors from ONC validation on chrome://policy.

ONC is a complex JSON policy. It has its own custom validator, so it probably doesn't make much sense to specify a schema for it in policy_templates.json.

However, the ONC validator just logs errors, they're not piped through to chrome://policy.

Maybe we can just queue all warnings/errors to a string array in ONC validator and have NetworkConfigurationPolicyHandler pipe them through to the PolicyErrorMap?

Note that the ONC validator is also used as a command line tool, see int main() in onc_validator.cc, which relies on logging to stdout/stderr.

 
Cc: olsen@chromium.org
Owner: ljusten@chromium.org

Comment 2 by pmarko@chromium.org, Jun 26 2018

Cc: hendrich@chromium.org emaxx@chromium.org
Labels: OS-Chrome
Owner: hendrich@chromium.org
Cc: ljusten@chromium.org
Project Member

Comment 5 by bugdroid1@chromium.org, Aug 10

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

commit f27ddf407afcfef726612fda7a20f8ada5fda880
Author: Alexander Hendrich <hendrich@chromium.org>
Date: Fri Aug 10 09:17:13 2018

Detailed ONC value validation

This CL updates the ONC Validator to not only log detailed
errors/warning messages, but also collect them for later use (e.g.,
reporting to policy server or displaying in chrome://policy).

DesignDoc: go/chromeos-robust-onc-policy

Bug: 794848,  855057 
Change-Id: Ie22ed887b3b298a8b7e92b175376f98d5f492dc3
Reviewed-on: https://chromium-review.googlesource.com/1116787
Commit-Queue: Alexander Hendrich <hendrich@chromium.org>
Reviewed-by: Pavol Marko <pmarko@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582099}
[modify] https://crrev.com/f27ddf407afcfef726612fda7a20f8ada5fda880/chrome/browser/chromeos/policy/configuration_policy_handler_chromeos.cc
[modify] https://crrev.com/f27ddf407afcfef726612fda7a20f8ada5fda880/chromeos/network/managed_network_configuration_handler_impl.cc
[modify] https://crrev.com/f27ddf407afcfef726612fda7a20f8ada5fda880/chromeos/network/managed_network_configuration_handler_unittest.cc
[modify] https://crrev.com/f27ddf407afcfef726612fda7a20f8ada5fda880/chromeos/network/onc/onc_utils.cc
[modify] https://crrev.com/f27ddf407afcfef726612fda7a20f8ada5fda880/chromeos/network/onc/onc_validator.cc
[modify] https://crrev.com/f27ddf407afcfef726612fda7a20f8ada5fda880/chromeos/network/onc/onc_validator.h
[modify] https://crrev.com/f27ddf407afcfef726612fda7a20f8ada5fda880/chromeos/network/onc/onc_validator_unittest.cc
[modify] https://crrev.com/f27ddf407afcfef726612fda7a20f8ada5fda880/chromeos/test/data/network/invalid_settings_with_repairs.json

Project Member

Comment 6 by bugdroid1@chromium.org, Aug 10

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

commit aa67ada171c1e9fd5089ea0880a9382c3a97991e
Author: Alexander Hendrich <hendrich@chromium.org>
Date: Fri Aug 10 17:26:03 2018

Display ONC value validation errors

This CL adds detailed warning/errors messages from the ONC value
validation to chrome://policy.

Bug:  855057 
Change-Id: I6d6bca90d074bc6b5c6be3b416fe25f216b5b7cb
Reviewed-on: https://chromium-review.googlesource.com/1118555
Reviewed-by: Pavol Marko <pmarko@chromium.org>
Reviewed-by: Lutz Justen <ljusten@chromium.org>
Commit-Queue: Alexander Hendrich <hendrich@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582224}
[modify] https://crrev.com/aa67ada171c1e9fd5089ea0880a9382c3a97991e/chrome/browser/chromeos/policy/configuration_policy_handler_chromeos.cc
[modify] https://crrev.com/aa67ada171c1e9fd5089ea0880a9382c3a97991e/components/policy_strings.grdp

Project Member

Comment 7 by bugdroid1@chromium.org, Aug 16

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

commit b6170513ec3ac3559618ed970f621be4540c61e1
Author: Alexander Hendrich <hendrich@chromium.org>
Date: Thu Aug 16 10:06:28 2018

Fix order of path_.pop_back() and AddValidationIssue() in ONC validator

This CL fixes the order of path_.pop_back() and AddValidationIssue() on
multiple occasions. The path is used in AddValidationIssue() and
therefore, the current field name should only be removed from the path
after the call to AddValidationIssue().

Bug: 794848,  855057 
Change-Id: I94f0cb67501fdf7625b6ae1820d8c602d462598a
Reviewed-on: https://chromium-review.googlesource.com/1174115
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: Alexander Hendrich <hendrich@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583593}
[modify] https://crrev.com/b6170513ec3ac3559618ed970f621be4540c61e1/chromeos/network/onc/onc_validator.cc

Status: Fixed (was: Assigned)
Status: Assigned (was: Fixed)
Hi Alexander,

Could you please confirm that the correct error message is displaying in chrome://policy (see attached screenshot) for ONC policy:

Network configuration failed to be parsed.

Chrome OS: 11021.5.0
Chrome: 70.0.3538.7
Device: Robo360
Screenshot 2018-09-06 at 10.10.09 AM.png
155 KB View Download
yes, this is correct.
thanks for testing!
Status: Verified (was: Assigned)
Marked as "Verified", thanks!

Sign in to add a comment