Merge pull request #5113 from Iristyle/ticket/stable/PUP-6499-puppet-…

…resource-user-fails-on-French-Windows

(PUP-6499) Ensure Windows ADSI strings UTF-8

lib/puppet/util/windows/adsi.rb

@@ -194,7 +194,10 @@ def self.logon(name, password)
     end
 
     def [](attribute)
-      native_user.Get(attribute)
+      value = native_user.Get(attribute)
+      # Rubys WIN32OLE errantly converts UTF-16 values to Encoding.default_external
+      return value.encode(Encoding::UTF_8) if value.is_a?(String)
+      value
     end
 
     def []=(attribute, value)
@@ -244,7 +247,8 @@ def groups
       # https://msdn.microsoft.com/en-us/library/aa746342.aspx
       # WIN32OLE objects aren't enumerable, so no map
       groups = []
-      native_user.Groups.each {|g| groups << g.Name} rescue nil
+      # Rubys WIN32OLE errantly converts UTF-16 values to Encoding.default_external
+      native_user.Groups.each {|g| groups << g.Name.encode(Encoding::UTF_8)} rescue nil
       groups
     end
 
@@ -330,6 +334,10 @@ def self.current_user_name
       user_name
     end
 
+    def self.current_user_sid
+      Puppet::Util::Windows::SID.name_to_sid_object(current_user_name)
+    end
+
     def self.exists?(name_or_sid)
       well_known = false
       if (sid = Puppet::Util::Windows::SID.name_to_sid_object(name_or_sid))
@@ -363,7 +371,8 @@ def self.each(&block)
 
       users = []
       wql.each do |u|
-        users << new(u.name)
+        # Rubys WIN32OLE errantly converts UTF-16 values to Encoding.default_external
+        users << new(u.name.encode(Encoding::UTF_8))
       end
 
       users.each(&block)
@@ -455,7 +464,8 @@ def remove_member_sids(*sids)
     def members
       # WIN32OLE objects aren't enumerable, so no map
       members = []
-      native_group.Members.each {|m| members << m.Name}
+      # Rubys WIN32OLE errantly converts UTF-16 values to Encoding.default_external
+      native_group.Members.each {|m| members << m.Name.encode(Encoding::UTF_8)}
       members
     end
 
@@ -524,7 +534,8 @@ def self.each(&block)
 
       groups = []
       wql.each do |g|
-        groups << new(g.name)
+        # Rubys WIN32OLE errantly converts UTF-16 values to Encoding.default_external
+        groups << new(g.name.encode(Encoding::UTF_8))
       end
 
       groups.each(&block)

lib/puppet/util/windows/process.rb

@@ -269,6 +269,11 @@ def set_environment_variable(name, val)
   end
   module_function :set_environment_variable
 
+  def get_system_default_ui_language
+    GetSystemDefaultUILanguage()
+  end
+  module_function :get_system_default_ui_language
+
   # Returns whether or not the OS has the ability to set elevated
   # token information.
   #
@@ -476,4 +481,9 @@ class OSVERSIONINFO < FFI::Struct
   ffi_lib :kernel32
   attach_function_private :GetVersionExW,
     [:pointer], :win32_bool
+
+  # https://msdn.microsoft.com/en-us/library/windows/desktop/dd318123(v=vs.85).aspx
+  # LANGID GetSystemDefaultUILanguage(void);
+  ffi_lib :kernel32
+  attach_function_private :GetSystemDefaultUILanguage, [], :word
 end

spec/integration/type/file_spec.rb

@@ -722,11 +722,11 @@ def build_path(dir)
 
       FileUtils.mkdir_p(srcdir)
       FileUtils.mkdir_p(dstdir)
-      
+
       srcfile = File.join(srcdir, "file.src")
       cpyfile = File.join(dstdir, "file.src")
       ignfile = File.join(srcdir, "file.ign")
-      
+
       File.open(srcfile, "w") { |f| f.puts "don't ignore me" }
       File.open(ignfile, "w") { |f| f.puts "you better ignore me" }
 
@@ -735,15 +735,15 @@ def build_path(dir)
                              :name => srcdir,
                              :ensure => 'directory',
                              :mode => '0755',)
-      
+
       catalog.add_resource described_class.new(
                              :name => dstdir,
                              :ensure => 'directory',
                              :mode => "755",
                              :source => srcdir,
                              :recurse => true,
                              :ignore => '*.ign',)
-      
+
       catalog.apply
       expect(Puppet::FileSystem.exist?(srcdir)).to be_truthy
       expect(Puppet::FileSystem.exist?(dstdir)).to be_truthy
@@ -1393,9 +1393,8 @@ def expects_at_least_one_inherited_system_ace_grants_full_access(path)
       describe "when processing SYSTEM ACEs" do
         before do
           @sids = {
-            :current_user => Puppet::Util::Windows::SID.name_to_sid(Puppet::Util::Windows::ADSI::User.current_user_name),
+            :current_user => Puppet::Util::Windows::ADSI::User.current_user_sid.sid,
             :system => Puppet::Util::Windows::SID::LocalSystem,
-            :guest => Puppet::Util::Windows::SID.name_to_sid("Guest"),
             :users => Puppet::Util::Windows::SID::BuiltinUsers,
             :power_users => Puppet::Util::Windows::SID::PowerUsers,
             :none => Puppet::Util::Windows::SID::Nobody
@@ -1415,8 +1414,8 @@ def expects_at_least_one_inherited_system_ace_grants_full_access(path)
 
           describe "when permissions are not insync?" do
             before :each do
-              @file[:owner] = 'None'
-              @file[:group] = 'None'
+              @file[:owner] = @sids[:none]
+              @file[:group] = @sids[:none]
             end
 
             it "preserves the inherited SYSTEM ACE for an existing file" do
@@ -1503,8 +1502,8 @@ def grant_everyone_full_access(path)
 
           describe "when permissions are not insync?" do
             before :each do
-              @directory[:owner] = 'None'
-              @directory[:group] = 'None'
+              @directory[:owner] = @sids[:none]
+              @directory[:group] = @sids[:none]
             end
 
             it "preserves the inherited SYSTEM ACEs for an existing directory" do
@@ -1726,7 +1725,7 @@ def pretty_transaction_error(transaction)
         catalog.apply
         expect(Puppet::FileSystem).to be_directory(copy)
       end
-    
+
       it "should copy the link itself if :links => manage" do
         catalog.add_resource described_class.new(
           :name => target,
@@ -1746,7 +1745,7 @@ def pretty_transaction_error(transaction)
         expect(Dir.entries(link)).to eq(Dir.entries(copy))
       end
     end
-  
+
     context "and the recurse attribute is true" do
       it "should recursively copy the directory if :links => follow" do
         catalog.add_resource described_class.new(

spec/integration/util/windows/adsi_spec.rb

@@ -0,0 +1,99 @@
+#!/usr/bin/env ruby
+require 'spec_helper'
+require 'puppet/util/windows'
+
+describe Puppet::Util::Windows::ADSI::User,
+  :if => Puppet.features.microsoft_windows? do
+
+  describe ".initialize" do
+    it "cannot reference BUILTIN accounts like SYSTEM due to WinNT moniker limitations" do
+      system = Puppet::Util::Windows::ADSI::User.new('SYSTEM')
+      # trying to retrieve COM object should fail to load with a localized version of:
+      # ADSI connection error: failed to parse display name of moniker `WinNT://./SYSTEM,user'
+      #     HRESULT error code:0x800708ad
+      #           The user name could not be found.
+      # Matching on error code alone is sufficient
+      expect { system.native_user }.to raise_error(/0x800708ad/)
+    end
+  end
+
+  describe '.each' do
+    it 'should return a list of users with UTF-8 names' do
+      begin
+        original_codepage = Encoding.default_external
+        Encoding.default_external = Encoding::CP850 # Western Europe
+
+        Puppet::Util::Windows::ADSI::User.each do |user|
+          expect(user.name.encoding).to be(Encoding::UTF_8)
+        end
+      ensure
+        Encoding.default_external = original_codepage
+      end
+    end
+  end
+
+  describe '.[]' do
+    it 'should return string attributes as UTF-8' do
+      administrator = Puppet::Util::Windows::ADSI::User.new('Administrator')
+      expect(administrator['Description'].encoding).to eq(Encoding::UTF_8)
+    end
+  end
+
+  describe '.groups' do
+    it 'should return a list of groups with UTF-8 names' do
+      begin
+        original_codepage = Encoding.default_external
+        Encoding.default_external = Encoding::CP850 # Western Europe
+
+
+        # lookup by English name Administrator is OK on localized Windows
+        administrator = Puppet::Util::Windows::ADSI::User.new('Administrator')
+        administrator.groups.each do |name|
+          expect(name.encoding).to be(Encoding::UTF_8)
+        end
+      ensure
+        Encoding.default_external = original_codepage
+      end
+    end
+  end
+end
+
+describe Puppet::Util::Windows::ADSI::Group,
+  :if => Puppet.features.microsoft_windows? do
+
+  let (:administrator_bytes) { [1, 2, 0, 0, 0, 0, 0, 5, 32, 0, 0, 0, 32, 2, 0, 0] }
+  let (:administrators_principal) { Puppet::Util::Windows::SID::Principal.lookup_account_sid(administrator_bytes) }
+
+  describe '.each' do
+    it 'should return a list of groups with UTF-8 names' do
+      begin
+        original_codepage = Encoding.default_external
+        Encoding.default_external = Encoding::CP850 # Western Europe
+
+
+        Puppet::Util::Windows::ADSI::Group.each do |group|
+          expect(group.name.encoding).to be(Encoding::UTF_8)
+        end
+      ensure
+        Encoding.default_external = original_codepage
+      end
+    end
+  end
+
+  describe '.members' do
+    it 'should return a list of members with UTF-8 names' do
+      begin
+        original_codepage = Encoding.default_external
+        Encoding.default_external = Encoding::CP850 # Western Europe
+
+        # lookup by English name Administrators is not OK on localized Windows
+        admins = Puppet::Util::Windows::ADSI::Group.new(administrators_principal.account)
+        admins.members.each do |name|
+          expect(name.encoding).to be(Encoding::UTF_8)
+        end
+      ensure
+        Encoding.default_external = original_codepage
+      end
+    end
+  end
+end