Merge branch 'jfm/cert_update' of github.com:chef/chef into jfm/cert_update

4 files changed, 24 insertions, 201 deletions

diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md
index 6c0b4b902c..66af43ed18 100644
--- a/RELEASE_NOTES.md
+++ b/RELEASE_NOTES.md
@@ -11,7 +11,7 @@ This section serves to track things we should later document here for 17.0
 - Remove ability to run client as a service on Windows - https://github.com/chef/chef/pull/10928
 - Knife Org commands from knife-opc are now part of chef itself - https://github.com/chef/chef/pull/10187
 - Chef packages on *nix now create the /etc/chef directory and subdirectories to make getting started easier - https://github.com/chef/chef/pull/11158 / https://github.com/chef/chef/pull/11173
-- lpar_no and wpar_no in AIX Virtualizatin plugin are now Integers - https://github.com/chef/ohai/pull/1647
+- lpar_no and wpar_no in AIX Virtualization plugin are now Integers - https://github.com/chef/ohai/pull/1647
 
 ### Infra Language Improvements
 
diff --git a/lib/chef/resource/windows_certificate.rb b/lib/chef/resource/windows_certificate.rb
index 0c9e245269..2f3c87d146 100644
--- a/lib/chef/resource/windows_certificate.rb
+++ b/lib/chef/resource/windows_certificate.rb
@@ -80,6 +80,8 @@ class Chef
         description: "Use the `CurrentUser` store instead of the default `LocalMachine` store. Note: Prior to #{ChefUtils::Dist::Infra::CLIENT}. 16.10 this property was ignored.",
         default: false
 
+      deprecated_property_alias :cert_path, :output_path, "The cert_path property was renamed output_path in the 17.0 release of this cookbook. Please update your cookbooks to use the new property name."
+
       # lazy used to set default value of sensitive to true if password is set
       property :sensitive, [TrueClass, FalseClass],
         description: "Ensure that sensitive resource data is not logged by the #{ChefUtils::Dist::Infra::CLIENT}.",
@@ -92,11 +94,9 @@ class Chef
 
       property :output_path, String,
         description: "A path on the node where a certificate object (pfx, pem, cer, key, etc) can be exported to.",
-        introduced: "16.10"
-
-      action :create do
-        description "Creates or updates a certificate."
+        introduced: "17.0"
 
+      action :create, description: "Creates or updates a certificate." do
         ext = get_file_extension(new_resource.source)
 
         # PFX certificates contains private keys and we import them with some other approach
@@ -104,8 +104,7 @@ class Chef
       end
 
       # acl_add is a modify-if-exists operation : not idempotent
-      action :acl_add do
-        description "Adds read-only entries to a certificate's private key ACL."
+      action :acl_add, description: "Adds read-only entries to a certificate's private key ACL." do
 
         if ::File.exist?(new_resource.source)
           hash = "$cert.GetCertHashString()"
@@ -128,9 +127,7 @@ class Chef
         end
       end
 
-      action :delete do
-        description "Deletes a certificate."
-
+      action :delete, description: "Deletes a certificate." do
         cert_obj = fetch_cert
 
         if cert_obj
@@ -142,10 +139,8 @@ class Chef
         end
       end
 
-      action :fetch do
-        description "Fetches a certificate."
-
-        if !new_resource.output_path
+      action :fetch, description: "Fetches a certificate." do
+        unless new_resource.output_path
           raise Chef::Exceptions::ResourceNotFound, "You must include an output_path parameter when calling the fetch action"
         end
 
@@ -164,9 +159,7 @@ class Chef
         end
       end
 
-      action :verify do
-        description "Verifies a certificate and logs the result"
-
+      action :verify, description: "Verifies a certificate and logs the result" do
         out = verify_cert
         if !!out == out
           out = out ? "Certificate is valid" : "Certificate not valid"
@@ -185,7 +178,7 @@ class Chef
           store.add(cert_obj)
         end
 
-        def add_pfx_cert(path) #expecting a path object here.
+        def add_pfx_cert(path)
           exportable = new_resource.exportable ? 1 : 0
           store = ::Win32::Certstore.open(new_resource.store_name, store_location: native_cert_location)
           store.add_pfx(path, new_resource.pfx_password, exportable)
@@ -207,11 +200,11 @@ class Chef
         end
 
         def fetch_key
-          require "openssl"
+          require "openssl" unless defined?(OpenSSL)
           file_name = ::File.basename(new_resource.output_path, ::File.extname(new_resource.output_path))
           directory = ::File.dirname(new_resource.output_path)
           pfx_file = file_name + ".pfx"
-          new_pfx_output_path = ::File.join(Chef::FileCache.create_cache_path('pfx_files'), pfx_file)
+          new_pfx_output_path = ::File.join(Chef::FileCache.create_cache_path("pfx_files"), pfx_file)
           powershell_exec(pfx_ps_cmd(resolve_thumbprint(new_resource.source), store_location: ps_cert_location, store_name: new_resource.store_name, output_path: new_pfx_output_path, password: new_resource.pfx_password ))
           pkcs12 = OpenSSL::PKCS12.new(::File.binread(new_pfx_output_path), new_resource.pfx_password)
           f = ::File.open(new_resource.output_path, "w")
@@ -226,12 +219,11 @@ class Chef
           if type == "file"
             ::File.extname(file_name)
           elsif type == "url"
-            require 'open-uri'
+            require "open-uri" unless defined?(OpenURI)
             uri = URI.parse(file_name)
             output_file = ::File.basename(uri.path)
             ::File.extname(output_file)
           end
-
         end
 
         def get_file_name(path_name)
@@ -240,20 +232,19 @@ class Chef
           if type == "file"
             ::File.extname(path_name)
           elsif type == "url"
-            require 'open-uri'
+            require "open-uri" unless defined?(OpenURI)
             uri = URI.parse(path_name)
             ::File.basename(uri.path)
           end
-
         end
 
         # did I get passed a file, a url, or a mistake?
         def url_or_file?(source)
-          require 'uri'
+          require "uri" unless defined?(URI)
 
           uri = URI.parse(source)
 
-          if source == uri.kind_of?(URI::HTTP) || uri.kind_of?(URI::HTTPS)
+          if source == uri.is_a?(URI::HTTP) || uri.is_a?(URI::HTTPS)
             "url"
           elsif ::File.file?(source)
             "file"
@@ -276,6 +267,7 @@ class Chef
 
         def resolve_thumbprint(thumbprint)
           return thumbprint if valid_thumbprint?(thumbprint)
+
           powershell_exec!(get_thumbprint(new_resource.store_name, ps_cert_location, new_resource.source)).result
         end
 
@@ -303,10 +295,9 @@ class Chef
 
         def pfx_ps_cmd(thumbprint, store_location: "LocalMachine", store_name: "My", output_path:, password: )
           <<-CMD
-            $mypwd = ConvertTo-SecureString -String "#{password}" -Force -AsPlainText
+            $my_pwd = ConvertTo-SecureString -String "#{password}" -Force -AsPlainText
             $cert = Get-ChildItem -path cert:\\#{store_location}\\#{store_name} -Recurse | Where { $_.Thumbprint -eq "#{thumbprint.upcase}" }
-            # | Export-PfxCertificate -FilePath #{output_path} -Password $mypwd
-            Export-PfxCertificate -Cert $cert -FilePath "#{output_path}" -Password $mypwd
+            Export-PfxCertificate -Cert $cert -FilePath "#{output_path}" -Password $my_pwd
           CMD
         end
 
@@ -396,12 +387,12 @@ class Chef
               ::File.exist?(new_resource.source)
               contents = ::File.binread(new_resource.source)
             rescue => exception
-              message = "Unable to load the certificate object from the specified localpath : #{new_resource.source}\n"
+              message = "Unable to load the certificate object from the specified local path : #{new_resource.source}\n"
               message << exception.message
               raise Chef::Exceptions::FileNotFound, message
             end
           elsif type == "url"
-            require 'uri'
+            require "uri" unless defined?(URI)
             uri = URI(new_resource.source)
             state = uri.is_a?(URI::HTTP) && !uri.host.nil? ? true : false
             if state
diff --git a/spec/functional/resource/windows_certificate2_spec.rb b/spec/functional/resource/windows_certificate2_spec.rb
deleted file mode 100644
index 7852a88155..0000000000
--- a/spec/functional/resource/windows_certificate2_spec.rb
+++ /dev/null
@@ -1,168 +0,0 @@
-# Author: Nimesh Patni (nimesh.patni@msystechnologies.com)
-# Copyright:: Copyright (c) Chef Software Inc.
-# License: Apache License, Version 2.0
-#
-# Licensed under the Apache License, Version 2.0 (the "License");
-# you may not use this file except in compliance with the License.
-# You may obtain a copy of the License at
-#
-#     http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing, software
-# distributed under the License is distributed on an "AS IS" BASIS,
-# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-# See the License for the specific language governing permissions and
-# limitations under the License.
-#
-
-require "spec_helper"
-require "chef/mixin/powershell_exec"
-require "chef/resource/windows_certificate"
-
-describe Chef::Resource::WindowsCertificate, :windows_only do
-  include Chef::Mixin::PowershellExec
-
-  def create_store(store_location: "LocalMachine", store_name: store)
-    powershell_exec <<~EOC
-      New-Item -Path Cert:\\#{store_location}\\#{store_name}
-    EOC
-  end
-
-  def delete_store(store_location: "LocalMachine", store_name: store)
-    powershell_exec <<~EOC
-      Remove-Item -Path Cert:\\#{store_location}\\#{store_name} -Recurse
-    EOC
-  end
-
-  def certificate_count(store_location: "LocalMachine", store_name: store)
-    powershell_exec(<<~EOC).result.to_i
-      (Get-ChildItem -Force -Path Cert:\\#{store_location}\\#{store_name} | measure).Count
-    EOC
-  end
-
-  let(:password) { "P@ssw0rd!" }
-  let(:store) { "Chef-Functional-Test" }
-  # let(:store_name) { "MY" }
-  let(:store_location) { "LocalMachine" }
-  let(:download_cert_url) { "https://testingchef.blob.core.windows.net/files/test.cer?sv=2020-02-10&ss=b&srt=sco&sp=rlax&se=2022-03-20T01:20:15Z&st=2021-03-19T17:20:15Z&spr=https&sig=nMmvTTXp%2Fn0%2FYizBV8BzhjRJ%2Bmk%2BxYZ9529yOfqDxjQ%3D" }
-  let(:cert_output_path) { ::File.join(Chef::Config[:file_cache_path], "output.cer") }
-  let(:pfx_output_path) { ::File.join(Chef::Config[:file_cache_path], "output.pfx") }
-  let(:key_output_path) { ::File.join(Chef::Config[:file_cache_path], "output.key") }
-  let(:cer_path) { File.join(CHEF_SPEC_DATA, "windows_certificates", "test.cer") }
-  let(:base64_path) { File.join(CHEF_SPEC_DATA, "windows_certificates", "base64_test.cer") }
-  let(:pem_path) { File.join(CHEF_SPEC_DATA, "windows_certificates", "test.pem") }
-  let(:p7b_path) { File.join(CHEF_SPEC_DATA, "windows_certificates", "test.p7b") }
-  let(:pfx_path) { File.join(CHEF_SPEC_DATA, "windows_certificates", "test.pfx") }
-  let(:tests_thumbprint) { "e45a4a7ff731e143cf20b8bfb9c7c4edd5238bb3" }
-  let(:other_cer_path) { File.join(CHEF_SPEC_DATA, "windows_certificates", "othertest.cer") }
-  let(:others_thumbprint) { "6eae1deefaf59daf1a97c9ceeff39c98b3da38cb" }
-  let(:p7b_thumbprint) { "f867e25b928061318ed2c36ca517681774b06260" }
-  let(:p7b_nested_thumbprint) { "dc395eae6be5b69951b8b6e1090cfc33df30d2cd" }
-
-  let(:resource) do
-    run_context = Chef::RunContext.new(Chef::Node.new, {}, Chef::EventDispatch::Dispatcher.new)
-    Chef::Resource::WindowsCertificate.new("ChefFunctionalTest", run_context).tap do |r|
-      r.store_name = store
-    end
-  end
-
-  before do
-    # Bypass validation of the store name so we can use a fake test store.
-    allow_any_instance_of(Chef::Mixin::ParamsValidate)
-      .to receive(:_pv_equal_to)
-      .with({ store_name: store }, :store_name, anything)
-      .and_return(true)
-
-      create_store
-
-  end
-
-  after { delete_store }
-
-  describe "action: verify" do
-    it "fails with no certificates in the store" do
-      # expect(Chef::Log).to receive(:info).with("Certificate not found")
-
-      # resource.source = tests_thumbprint
-      # resource.run_action(:verify)
-
-      # expect(resource).not_to be_updated_by_last_action
-    end
-  end
-
-  describe "action: fetch" do
-    # context "with no certificate in the store" do
-    #   it "throws an error with no certificates in the store" do
-    #     expect(Chef::Log).not_to receive(:info)
-    #     resource.source = others_thumbprint
-    #     resource.output_path = cert_output_path
-    #     expect { resource.run_action :fetch }.to raise_error(ArgumentError)
-    #   end
-    # end
-
-    # context "with a certificate in the store" do
-    #   before do
-    #     resource.source = cer_path
-    #     resource.run_action(:create)
-    #   end
-
-    #   it "succeeds with a valid thumbprint" do
-    #     resource.source = tests_thumbprint
-    #     local_output_path = ::File.join(Chef::Config[:file_cache_path], "test.pem")
-    #     resource.output_path = local_output_path
-    #     resource.run_action(:fetch)
-    #     expect(File.exist?(local_output_path)).to be_truthy
-    #   end
-
-    #   it "fails with an invalid thumbprint", :focus do
-    #     expect(Chef::Log).not_to receive(:info)
-
-    #     resource.source = others_thumbprint
-
-    #     Dir.mktmpdir do |dir|
-    #       path = File.join(dir, "test.pem")
-
-    #       resource.output_path = path
-    #       expect { resource.run_action :fetch }.to raise_error(ArgumentError)
-    #     end
-    #   end
-    # end
-
-    context "with a pfx/pkcs12 object in the store" do
-      before do
-        resource.source = pfx_path
-        resource.pfx_password = password
-        resource.exportable = true
-        resource.run_action(:create)
-      end
-
-      # it "verfies" do
-      #   resource.source = tests_thumbprint
-      #   resource.run_action(:verify)
-      # end
-
-      it "exports a PFX file with a valid thumbprint", :focus do
-        resource.source = tests_thumbprint
-        resource.pfx_password = password
-        resource.output_path = pfx_output_path
-        resource.run_action(:fetch)
-        expect(File.exist?(pfx_output_path)).to be_truthy
-      end
-
-      # it "exports a key file with a valid thumbprint" do
-      #   resource.source = tests_thumbprint
-      #   resource.pfx_password = password
-      #   resource.output_path = key_output_path
-      #   resource.run_action(:fetch)
-      #   expect(File.exist?(key_output_path)).to be_truthy
-      # end
-
-      # it "throws an exception when output_path is not specified" do
-      #   resource.source = tests_thumbprint
-      #   resource.pfx_password = password
-      #   expect { resource.run_action :fetch }.to raise_error(::Chef::Exceptions::ResourceNotFound)
-      # end
-    end
-  end
-
-end
diff --git a/spec/functional/resource/windows_certificate_spec.rb b/spec/functional/resource/windows_certificate_spec.rb
index c504beb66c..5c336d216a 100644
--- a/spec/functional/resource/windows_certificate_spec.rb
+++ b/spec/functional/resource/windows_certificate_spec.rb
@@ -234,7 +234,7 @@ describe Chef::Resource::WindowsCertificate, :windows_only do
         resource.run_action(:create)
       end
 
-      it "succeeds with the main certificate's thumbprint", :focus do
+      it "succeeds with the main certificate's thumbprint" do
         expect(Chef::Log).to receive(:info).with("Certificate is valid")
 
         resource.source = p7b_thumbprint
@@ -310,7 +310,7 @@ describe Chef::Resource::WindowsCertificate, :windows_only do
         resource.run_action(:create)
       end
 
-      it "exports a PFX file with a valid thumbprint", :focus do
+      it "exports a PFX file with a valid thumbprint" do
         resource.source = tests_thumbprint
         resource.pfx_password = password
         resource.output_path = pfx_output_path