Skip to content

Commit

Permalink
minimally handle github issue with cpe_chrome windows
Browse files Browse the repository at this point in the history
Summary:
We don't actually use this setting internally for anything so we've never seen if it actually works. The short of it is there are some Chrome settings that need to be `:string` types in the registry that are actually just JSON strings.

The resource can't really do that without explicitly setting the setting to be a JSON string, for example:
```
node.default['cpe_chrome']['profile']['ManagedBookmarks'] = [
  {
    'name' => 'My Bookmark',
    'url' => 'https://google.com'
  },
  {
    'name' => 'Another Bookmark',
    'url' => 'https://google.com'
  }
].to_json
```

To maintain the API we have I will add support to render the appropriate `ManagedBookmarks` setting to the registry. Additionally, a unit test rendering the example setting in the [corresponding documentation](https://cloud.google.com/docs/chrome-enterprise/policies/?policy=ManagedBookmarks) is added to ensure the output is valid.

Reviewed By: chilcote

Differential Revision: D18846631

fbshipit-source-id: dc88d96a5150b61f574ae4bd28aae824e44c696b
  • Loading branch information
svmastersamurai authored and facebook-github-bot committed Dec 9, 2019
1 parent 0304dd1 commit 435f87c
Show file tree
Hide file tree
Showing 3 changed files with 89 additions and 9 deletions.
10 changes: 9 additions & 1 deletion itchef/cookbooks/cpe_chrome/libraries/chrome_windows.rb
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,6 @@ def self.chrome_reg_root
'ImportSavedPasswords' => :dword,
'ImportSearchEngine' => :dword,
'IncognitoModeAvailability' => :dword,
'ManagedBookmarks' => :multi_string,
'MaxConnectionsPerProxy' => :dword,
'MaxInvalidationFetchDelay' => :dword,
'MediaCacheSize' => :dword,
Expand Down Expand Up @@ -239,6 +238,15 @@ def self.chrome_reg_root
'URLWhitelist' => :string,
'VideoCaptureAllowedUrls' => :string,
}.freeze

# These keys can be passed in an array of dictionaries and the resource
# will call `.to_json` on them so that they actually work. Please confirm
# in the documentation that you are creating the necessary data structure.
JSONIFY_REG_KEYS = {
'Chrome' => {
'ManagedBookmarks' => :string,
},
}.freeze
end
end
end
29 changes: 28 additions & 1 deletion itchef/cookbooks/cpe_chrome/libraries/windows_chrome_setting.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,25 @@ def initialize(setting, suffix_reg_path = nil, forced = false)

reg_entry = construct_reg_key_path(setting.keys.first, suffix_reg_path)
@fullpath = reg_entry.keys.first.to_s
@path = @fullpath.split(@name).first.chop
@type = reg_entry.values.first

p = @fullpath.split(@name)
@path = if p.size == 1
p.first.split('\\')[0..-1].join('\\')
else
p.first.chop
end
end

attr_reader :path, :fullpath, :type, :data, :name, :scope, :forced

# This method will output a hash that can be consumed by the chef registry_key
# resource.
def to_chef_reg_provider
if in_json_key?
return { :name => @name, :type => @type, :data => @data.to_json }
end

# Chrome settings in the registry with multiple entries are laid out
# sequentially from [1...N]
if data_has_multiple_entries?
Expand Down Expand Up @@ -86,6 +96,11 @@ def construct_reg_key_path(key = @name, suffix_path = nil)
"#{CPE::ChromeManagement.chrome_reg_root}\\#{key}" =>
ENUM_REG_KEYS[key],
}
elsif in_json_key?
{
CPE::ChromeManagement.chrome_reg_root =>
JSONIFY_REG_KEYS[which_json_key][key],
}
elsif in_complex_key?
lookup_complex(key)
elsif !suffix_path.nil?
Expand Down Expand Up @@ -124,4 +139,16 @@ def which_complex_key
return 'Chrome' if COMPLEX_REG_KEYS['Chrome'].include?(@name)
return 'Recommended' if COMPLEX_REG_KEYS['Recommended'].include?(@name)
end

# Returns true if the setting is located in a registry key hash that should be
# a JSON value, false otherwise.
def in_json_key?(key = @name)
return true if JSONIFY_REG_KEYS['Chrome'].include?(key)
false
end

# Returns which JSON key the setting is located under.
def which_json_key
return 'Chrome' if JSONIFY_REG_KEYS['Chrome'].include?(@name)
end
end
59 changes: 52 additions & 7 deletions itchef/cookbooks/cpe_chrome/spec/windows_chrome_setting_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,22 +17,47 @@
# limitations under the License.
#

require 'json'
require_relative 'spec_helper'

RSpec.describe WindowsChromeSetting do
JSON_EXAMPLE_DATA = [
{
:toplevel_name => 'My managed bookmarks folder',
},
{
:url => 'google.com',
:name => 'Google',
},
{
:url => 'youtube.com',
:name => 'Youtube',
},
{
:name => 'Chrome links',
:children => [
{
:url => 'chromium.org',
:name => 'Chromium',
},
{
:url => 'dev.chromium.org',
:name => 'Chromium Developers',
},
],
},
].freeze
settings = {
'empty_setting' =>
{ 'ExtensionInstallBlacklist' => [] },
'empty_setting' => { 'ExtensionInstallBlacklist' => [] },
'doc_setting' => { 'AlternateErrorPagesEnabled' => [true] },
'json_setting' => { 'ManagedBookmarks' => JSON_EXAMPLE_DATA },
'example_setting' => {
'ExtensionInstallForcelist' =>
['ncfpggehkhmjpdjpefomjchjafhmbnai;bacon'],
'ExtensionInstallForcelist' => ['ncfpggehkhmjpdjpefomjchjafhmbnai;bacon'],
},
'doc_setting' =>
{ 'AlternateErrorPagesEnabled' => [true] },
}

extension_setting = {
'suffix_path' => 'ExtensionSettings\\ncfpggehkhmjpdjpefomjchjafhmbnai',
'suffix_path' => 'ExtensionSettings\ncfpggehkhmjpdjpefomjchjafhmbnai',
'setting' => { 'installation_mode' => 'removed' },
'expected' => {
'name' => 'installation_mode',
Expand Down Expand Up @@ -85,6 +110,17 @@
'\AlternateErrorPagesEnabled',
'path' => 'HKLM\Software\Policies\Google\Chrome',
},
'json_setting' => {
'header' => 'gh-issue-224 - A value that needs to be a JSON string',
'name' => 'ManagedBookmarks',
'type' => :string,
'data' => JSON_EXAMPLE_DATA,
'expected_class' => 'Array',
'fullpath' => 'HKLM\Software\Policies\Google\Chrome',
'fullpath_forced' => 'HKLM\Software\Policies\Google\Chrome',
'path' => 'HKLM\Software\Policies\Google\Chrome',
'output' => JSON_EXAMPLE_DATA.to_json,
},
}

shared_examples 'the chrome setting creates the key in HKLM' do
Expand Down Expand Up @@ -181,6 +217,15 @@
const_get(expected_results[setting]['expected_class'])
end
end
if expected_results[setting]['output']
subject do
WindowsChromeSetting.new(settings[setting]).
to_chef_reg_provider[:data]
end
context 'chef_to_reg_provider data output should equal' do
it { should eql expected_results[setting]['output'] }
end
end
end
end
end

0 comments on commit 435f87c

Please sign in to comment.