Skip to content

Commit

Permalink
Merge pull request #84 from rishichawda/shouldnot-raise-error-for-emp…
Browse files Browse the repository at this point in the history
…ty-certificate-in-get-call
  • Loading branch information
johnmccrae authored Sep 28, 2021
2 parents 133e497 + 0049fca commit 9d563c5
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 22 deletions.
19 changes: 18 additions & 1 deletion lib/win32/certstore.rb
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,17 @@ def get(certificate_thumbprint, store_name: @store_name, store_location: @store_
cert_get(certificate_thumbprint, store_name: store_name, store_location: store_location)
end

# Return `OpenSSL::X509` certificate object if present otherwise raise a "Certificate not found!" error
# @param request [thumbprint<string>] of certificate
# @return [Object] of certificates in OpenSSL::X509 format
def get!(certificate_thumbprint, store_name: @store_name, store_location: @store_location)
cert_pem = cert_get(certificate_thumbprint, store_name: store_name, store_location: store_location)

raise ArgumentError, "Unable to retrieve the certificate" if cert_pem.empty?

cert_pem
end

# Returns all the certificates in a store
# @param [nil]
# @return [Array] array of certificates list
Expand All @@ -103,7 +114,13 @@ def search(search_token)
# @param request[thumbprint<string>] of certificate
# @return [true, false] only true or false
def valid?(certificate_thumbprint, store_location: "", store_name: "")
cert_validate(certificate_thumbprint, store_location: store_location, store_name: store_name)
cert_validate(certificate_thumbprint, store_location: store_location, store_name: store_name).yield_self do |x|
if x.is_a?(TrueClass) || x.is_a?(FalseClass)
x
else
false
end
end
end

# To close and destroy pointer of open certificate store handler
Expand Down
11 changes: 3 additions & 8 deletions lib/win32/certstore/store_base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -92,13 +92,8 @@ def cert_get(certificate_thumbprint, store_name:, store_location:)
thumbprint = update_thumbprint(certificate_thumbprint)
cert_pem = get_cert_pem(thumbprint, store_name: store_name, store_location: store_location)
cert_pem = format_pem(cert_pem)
if cert_pem.empty?
raise ArgumentError, "Unable to retrieve the certificate"
end

unless cert_pem.empty?
build_openssl_obj(cert_pem)
end
cert_pem.empty? ? cert_pem : build_openssl_obj(cert_pem)
end

# Listing certificate of open certstore and return list in json
Expand Down Expand Up @@ -147,6 +142,8 @@ def cert_validate(certificate_thumbprint, store_location:, store_name:)
thumbprint = update_thumbprint(certificate_thumbprint)
cert_pem = get_cert_pem(thumbprint, store_name: store_name, store_location: store_location)
cert_pem = format_pem(cert_pem)
return "Certificate not found" if cert_pem.empty?

verify_certificate(cert_pem)
end

Expand Down Expand Up @@ -223,8 +220,6 @@ def update_thumbprint(certificate_thumbprint)

# Verify OpenSSL::X509::Certificate object
def verify_certificate(cert_pem)
return "Certificate not found" if cert_pem.empty?

valid_duration?(build_openssl_obj(cert_pem))
end

Expand Down
21 changes: 20 additions & 1 deletion spec/win32/functional/win32/certstore_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,29 @@
@store.get(thumbprint, store_location: CERT_SYSTEM_STORE_CURRENT_USER, store_name: "My")
end

# passing invalid thumbprint
it "returns nil if certificate not found" do
thumbprint = "b1bc968bd4f49d622aa89a81f2150152a41d829cab"
cert_obj = @store.get(thumbprint, store_location: CERT_SYSTEM_STORE_CURRENT_USER, store_name: "My")
expect(cert_obj).to be_empty
end
end

describe "#get!" do
before { add_cert }
let(:cert_pem) { File.read('.\spec\win32\assets\GlobalSignRootCA.pem') }

# passing valid thumbprint
it "returns the certificate_object if found" do
thumbprint = "b1bc968bd4f49d622aa89a81f2150152a41d829c"
expect(@store).to receive(:cert_get).with(thumbprint, store_location: CERT_SYSTEM_STORE_CURRENT_USER, store_name: "My").and_return(cert_pem)
@store.get!(thumbprint, store_location: CERT_SYSTEM_STORE_CURRENT_USER, store_name: "My")
end

# passing invalid thumbprint
it "returns ArgumentError if certificate not found" do
thumbprint = "b1bc968bd4f49d622aa89a81f2150152a41d829cab"
expect { @store.get(thumbprint, store_location: CERT_SYSTEM_STORE_CURRENT_USER, store_name: "My") }.to raise_error(ArgumentError)
expect { @store.get!(thumbprint, store_location: CERT_SYSTEM_STORE_CURRENT_USER, store_name: "My") }.to raise_error(ArgumentError)
end
end

Expand Down
28 changes: 16 additions & 12 deletions spec/win32/unit/certstore_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,10 @@
before(:each) do
allow_any_instance_of(certbase).to receive(:get_cert_pem).and_return("")
end
it "raises ArgumentError" do
it "returns empty string" do
store = certstore.open(store_name)
expect { store.get(thumbprint, store_location: CERT_SYSTEM_STORE_CURRENT_USER, store_name: store_name) }.to raise_error(ArgumentError, "Unable to retrieve the certificate")
cert_obj = store.get(thumbprint, store_location: CERT_SYSTEM_STORE_CURRENT_USER, store_name: store_name)
expect(cert_obj).to be_empty
end
end

Expand Down Expand Up @@ -259,9 +260,10 @@
before(:each) do
allow_any_instance_of(certbase).to receive(:get_cert_pem).and_return("")
end
it "raises Error" do
it "returns empty string" do
store = certstore.open(store_name)
expect { store.get(thumbprint, store_location: CERT_SYSTEM_STORE_CURRENT_USER, store_name: store_name) }.to raise_error(ArgumentError, "Unable to retrieve the certificate")
cert_obj = store.get(thumbprint, store_location: CERT_SYSTEM_STORE_CURRENT_USER, store_name: store_name)
expect(cert_obj).to be_empty
end
end
end
Expand Down Expand Up @@ -298,9 +300,9 @@
before(:each) do
allow_any_instance_of(certbase).to receive(:get_cert_pem).and_return("")
end
it "returns Certificate not found" do
it "returns false" do
store = certstore.open(store_name)
expect(store.valid?(thumbprint)).to eql("Certificate not found")
expect(store.valid?(thumbprint)).to eql(false)
end
end

Expand Down Expand Up @@ -597,9 +599,10 @@
before(:each) do
allow_any_instance_of(certbase).to receive(:get_cert_pem).and_return("")
end
it "returns nil" do
it "returns empty string" do
store = certstore.open(store_name, store_location: store_location)
expect { store.get(thumbprint, store_location: CERT_SYSTEM_STORE_CURRENT_USER, store_name: store_name) }.to raise_error(ArgumentError, "Unable to retrieve the certificate")
cert_obj = store.get(thumbprint, store_location: CERT_SYSTEM_STORE_CURRENT_USER, store_name: store_name)
expect(cert_obj).to be_empty
end
end

Expand Down Expand Up @@ -732,9 +735,10 @@
before(:each) do
allow_any_instance_of(certbase).to receive(:get_cert_pem).and_return("")
end
it "returns nil" do
it "returns empty string" do
store = certstore.open(store_name, store_location: store_location)
expect { store.get(thumbprint, store_location: CERT_SYSTEM_STORE_CURRENT_USER, store_name: store_name) }.to raise_error(ArgumentError, "Unable to retrieve the certificate")
cert_obj = store.get(thumbprint, store_location: CERT_SYSTEM_STORE_CURRENT_USER, store_name: store_name)
expect(cert_obj).to be_empty
end
end
end
Expand Down Expand Up @@ -775,9 +779,9 @@
before(:each) do
allow_any_instance_of(certbase).to receive(:get_cert_pem).and_return("")
end
it "returns Certificate not found" do
it "returns false" do
store = certstore.open(store_name, store_location: store_location)
expect(store.valid?(thumbprint)).to eql("Certificate not found")
expect(store.valid?(thumbprint)).to eql(false)
end
end

Expand Down

0 comments on commit 9d563c5

Please sign in to comment.